fix(ioc): drop type-lie in eager-DI provider construction#9
Merged
Conversation
… type-lie Before: ProvidersGroup had gitlab_provider + github_provider factories that were both eagerly resolved by current_provider's DI (modern-di's Factory.resolve() eagerly resolves all provider_kwargs, confirmed in factory.py:147). When provider=github, _build_gitlab_provider was called with settings.project_id=None and constructed GitLabProvider(project_id=None) — a `int`-typed field holding None (suppressed with `# ty: ignore`). Harmless today because no method references self.project_id at construction, but trivially broken if someone adds a __post_init__ validator or eager field access. After: drop the gitlab_provider/github_provider per-provider factories. _build_current_provider takes both clients directly and constructs only the active provider, narrowing settings.repo/project_id with asserts that document the validator's invariant. The unused client still resolves (eager Factory) but httpx2 connection pools are lazy so no real cost. Also consolidates _close_gitlab_provider + _close_github_provider into one _close_client finalizer attached to each client factory's cache_settings — the lifecycle target moved from the provider to the client it owns. Surfaced by the final code-review of the GitHub provider branch (PR #4); deferred at landing because the dogfood (PR #5/#6/#8) proved the type-lie path is currently unreachable. This PR closes the gap before someone makes it reachable. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
modern-di's
Factory.resolve()eagerly resolves allprovider_kwargs. The 0.3.xcurrent_providerfactory declared bothgitlab_providerandgithub_provideras kwargs, so both got constructed on every resolution. Whenprovider=github,_build_gitlab_provider(settings, gitlab_client)was called withsettings.project_id=None→GitLabProvider(project_id=None)— a field typedintholdingNone, suppressed with# ty: ignore. Harmless today (no method touchesself.project_idat construction), but trivially broken if anyone adds a__post_init__validator or eager field access.This PR collapses the per-provider factories into a single
_build_current_providerthat takes both clients and constructs only the active provider, with asserts that narrow theOptionalfields and document theSettings._resolve_providervalidator invariant._build_gitlab_provider,_build_github_provider,_close_gitlab_provider,_close_github_providerProvidersGroup.gitlab_provider,ProvidersGroup.github_provider_close_clientattached to each client factory'scache_settings(lifecycle target moved from provider to client)_build_current_providernow takes(settings, gitlab_client, github_client); the unused client still resolves (eager Factory) but httpx2 connection pools are lazy420 tests pass, 100% coverage, lint clean — existing test_ioc.py tests resolve
current_providerand check the type, which still works under the new shape.Test plan
just test— 420 pass (same total, no test removed/added)just lint-ci— cleantest_container_resolves_github_provider_when_settings_provider_is_github+..._gitlab_..._is_gitlabconfirm both branches dispatch correctlybugfix/prefix triggers dogfood auto-tag (0.3.2 → 0.3.3)🤖 Generated with Claude Code